[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 바니 미션 제출합니다.#352
[🚀 사이클2 - 미션 (기물 확장 + DB 적용)] 바니 미션 제출합니다.#352verus-j merged 54 commits intowoowacourse:frombunnyfrom
Conversation
verus-j
left a comment
verus-j
left a comment
There was a problem hiding this comment.
안녕하세요 바니~
미션 진행하느라 고생하셨습니다. 피드백 남겨놨으니 확인부탁드려요.
DB 테이블 구조는 적절히 잘 구성하셨습니다. 트랜잭션에 대한 학습은 지금 수준으로 적당한 것 같습니다. 레벨2에서 좀 더 깊게 배우니 지금은 이정도 수준으로만 학습하셔도 충분합니다~ 저도 촉박한 일정 속에서 새로운 기술을 적용해야할때면 기능을 구현하는데 필요한 정도까지만 학습합니다. 아무래도 시간 제한이 있다보니 깊게는 학습할 시간이 없더라구요.
DB 테스트는 아무래도 레벨1에서 바로 학습하기 어려운 것 같네요 ㅠㅠ 테스트 작성의 어려움 때문에 TDD 흐름이 힘드셨던거 같은데 전 보통 테스트 작성 방법을 학습해봅니다. 보통은 라이브러리나 프레임워크 Docs에 테스트 작성 방법이 있어 항상 챙겨서 읽어보구요. 요즘은 GPT에게 물어봐서 테스트 작성 방법들을 학습한 후 TDD를 진행합니다.
PathGenerator와 BlockingPieceValidator 구조에 대한 피드백은 남겨놨으니 확인부탁드려요~
|
|
||
| import domain.path.Direction; | ||
|
|
||
| public class Palace { |
There was a problem hiding this comment.
모든 메서드를 정적 메서드로 만드셨네요. 객체를 활용하는 방법을 고민해보는 건 어떨까요?
There was a problem hiding this comment.
현재 Palace는 상태를 가지지 않고 고정된 범위를 계산하는 순수 로직만 담고 있어,
부수 효과가 없는 유틸리티 클래스로 두는 것이 단순하고 명확하다고 판단했습니다..!!!
다만 객체로 분리했을 때 ‘궁성’이라는 도메인 모델이 장기판 위에서 하나의 주체로서 역할을 가질 수 있다는 점에는 공감하고 있습니다. 특히 LinearPathGenerator가 정적 메서드에 의존하는 구조보다는, Palace 객체와 협력하는 구조가 더 객체지향적인 방향이라고 생각하긴 합니다..!
그럼에도 현재 단계에서는 궁의 범위나 규칙이 고정되어 있고, Camp에 따른 분기 역시 단순 조건 수준이라
객체로 분리했을 때 얻는 이점 대비 구조적인 복잡도가 조금 더 커질 수 있다고 느꼈습니다.
그래서 지금은 단순함을 유지하는 쪽을 우선 선택했는데요..!
이처럼 변경 가능성이 거의 없는 고정 영역 데이터도 일반적으로는 인스턴스로 만들어 관리하는 것이 더 권장되는지,
아니면 설계적으로 가져갈 수 있는 추가적인 이점이 있는지 궁금합니다.
제가 아직 체감하지 못한 설계상의 장점이 있다면 더 배우고 싶습니다 🙇♀️
There was a problem hiding this comment.
정적 메서드로 관리하면 global 함수처럼 다뤄져서 관련 로직들이 흩어질 수 있습니다. 아무래도 객체로 관계를 맺는 것보다 편하다보니 구조의 고민 없이 쉽게쉽게 사용하는 경향이 있어서 나중에 리팩터링 시 강한 의존관계로 어려움을 겪을 수도 있어요 ㅠㅠ
현재는 단순한 상황이여서 유틸리티 클래스로 관리해도 괜찮지만 코드가 발전하면서 궁성과 관련된 로직이 추가되면 연관없는 곳에 Palace를 사용한 후 궁성과 관련된 추가 로직을 작성하지 않도록 더 신경을 써야할 것 같네요.
| public abstract class JumpMoveStrategy implements MoveStrategy { | ||
| @Override | ||
| public List<Position> getPath(Position departure, Position destination) { | ||
| public class JumpPathGenerator { |
There was a problem hiding this comment.
PathGenerator들은 모두 유틸성 클래스가 된 것 같은데 OOP에 맞게 객체를 사용하도록 해보면 좋을 것 같아요. 깔끔하게 개별 MoveStrategy에 필요한 Generator를 인스턴스 변수로 만들어도 될 것 같네요. 상위 인터페이스인 MoveStrategy말고 개별 하위 클래스를 말한겁니다~
There was a problem hiding this comment.
다시 보니 베루스 말씀대로 모두 유틸성 클래스가 되어버렸네요ㅜㅜ
개별 하위 이동 전략 클래스들이 Generator를 인스턴스로 가지도록 수정했습니다!
|
|
||
| import java.util.List; | ||
|
|
||
| public class BlockingPieceValidator { |
There was a problem hiding this comment.
여기도 유틸성 클래스가 되어버렸는데 List를 감싼 랩퍼 클래스를 만들어 구현을 넘겨버리면 어떨까요? 그렇게 하면 좀 더 OOP 스러운 코드가 나올 것 같네요.
There was a problem hiding this comment.
List를 감싸는 랩퍼 클래스를 만들어 해당 객체에게 책임을 넘기도록 수정했습니다!!
조언 감사합니다 😊
| import java.sql.Connection; | ||
| import java.sql.SQLException; | ||
|
|
||
| public class TransactionManager { |
There was a problem hiding this comment.
메서드 구현이 너무 기네요 ㅠㅠ 메서드 추출로 정리해보면 좋을 것 같아요~
| import java.sql.*; | ||
| import java.util.Optional; | ||
|
|
||
| public class BoardDao { |
There was a problem hiding this comment.
들여쓰기가 2단계를 넘어가는 로직이 많은 것 같네요. 메서드 추출로 정리해보면 좋을 것 같아요~
| import view.InputView; | ||
|
|
||
| public class Application { | ||
| public static void main(String[] args) { |
There was a problem hiding this comment.
초기에 step2 브랜치에서 작업했어야 하는 내용을 실수로 frombunny 브랜치 기준으로 PR을 올리게 되었습니다. 죄송합니다 😭
이후 리뷰 반영 및 커밋 히스토리 정리를 위해, frombunny 기준으로 새로운 브랜치에서 커밋을 재구성한 뒤 step2 브랜치에 반영했습니다..!
|
안녕하세요, 베루스! 한 주의 시작부터 리뷰를 요청드리게 되었습니다🐰 |
verus-j
left a comment
verus-j
left a comment
There was a problem hiding this comment.
안녕하세요 바니~
2단계 미션 진행하느라 고생하셨습니다. 유틸리티 클래스가 많다고 느꼈는데 잘 정리해주신 것 같습니다. 코드가 깔끔하고 눈에 더 잘 들어오는 것 같네요. Palace에 대한 의문점을 공유해주셨는데 답변 남겼으니 확인부탁드려요~ 레벨1 동안 고생많으셨고 남은 우테코 생활도 화이팅입니다!
2 participants
안녕하세요, 베루스! 바니입니다🐰
레벨1의 마지막 사이클을 리뷰드리게 되었네요, 마지막까지 잘 부탁드립니다!
나름 이것저것 찾아보고 알아보면서 코드를 작성하였는데, 제대로 반영되었을진 모르겠습니다😭
바쁘신 와중에도 리뷰해주셔서 감사합니다!
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?어떤 부분에 집중하여 리뷰해야 할까요?
1. DB 테이블 구조 설계
장기판의 상태를 어떻게 저장해야 할지에 대해 고민이 많았습니다.
현재는 Board를 애그리거트 루트로 간주하고, 기물의 위치와 종류 정보를 중심의 테이블 구조를 설계했습니다..!
다만, 도메인 객체 구조와 DB 테이블 구조를 어디까지 일치시켜야 하는지
정규화 수준을 어느 정도까지 가져가는 것이 적절한지에 대해서는 확신이 없는 상태입니다.
2. JDBC 도입과 트랜잭션 관리에 대한 고민
이번 단계에서 처음으로 DB를 직접 연결하며, 기존에 JPA가 추상화해주던 영역(Connection, 트랜잭션 관리)을 직접 다루게 되었습니다. JDBC 레벨에서 Connection과 트랜잭션을 직접 다루다 보니 아래와 같은 지점에서 AI의 도움을 받았고, 아직은 완전히 제 것으로 소화하지 못한 상태입니다🥹
현재 TransactionManager를 통해 해당 책임을 분리하려고 시도했는데, 이 객체가 트랜잭션과 커넥션을 함께 관리하는 구조가 적절한지 확신이 없습니다. 트랜잭션 책임과 커넥션 관리 책임이 적절히 분리되어 있는지, 현재 구조에서 발생할 수 있는 문제점들에 대해서 말씀해주시면 감사하겠습니다..!
+) 미션을 진행하며 가장 어려웠던 점 중 하나는 제한된 시간 내에 어디까지 학습하고 구현에 반영해야 하는가였습니다. 추가 학습이 필요한 개념(순수 JDBC, 트랜잭션을 직접 관리하는 방법 등)을 마주했을 때, 원리를 완벽히 이해하고 코드를 짜고 싶었지만 현실적인 마감 기한과 충돌하며 타협해야 하는 순간들이 있었습니다. 모르는 기술이 나왔을 때 일단 동작하게 만드는 수준과 내부 원리를 완전히 파헤치는 수준 사이에서 어떻게 균형을 잡는 것이 좋을까요? 베루스는 촉박한 일정 속에 생소한 기술 요구사항을 받으셨을 때, 최소 학습 가이드라인이나 우선순위 설정 기준이 있으신지 궁금합니다.
3. TDD 진행의 어려움
DB 연결 로직이 추가되면서 테스트 환경을 어떻게 구축해야 할지, 실제 DB와의 의존성을 어떻게 분리해야 할지 판단이 서지 않아 이번 단계에서는 TDD를 원활하게 수행하지 못했습니다😭 DB에서 무엇을 테스트해야 하는가에 대한 막막함이 컸는데, 이런 상황에서 베루스는 어떤 식으로 테스트 리듬을 유지하시는지 궁금합니다..!
4. 기물 전략 구조 변경
기존의 복잡했던 이동 전략 구조를 가독성과 재사용성 중심으로 리팩터링했습니다.
기물은 자신의 이동 규칙에만 집중하도록 하고, 좌표 계산 및 경로 장애물 검증 로직은 PathGenerator와 BlockingPieceValidator로 분리했습니다. 이를 통해 책임을 분리하고 중복을 줄이고자 했습니다.
다만, static 유틸리티로 분리한 것이 적절한 선택인지, 현재 구조가 객체 간 협력 관점에서 자연스러운지
에 대해서는 확신이 없는 상태입니다.
수정한 구조는 어떤지 피드백해주시면 감사하겠습니다!